Skip to content

Fix Node Registration#481

Merged
lo-simon merged 4 commits intosony:masterfrom
lo-simon:fix-node-registration
Mar 5, 2026
Merged

Fix Node Registration#481
lo-simon merged 4 commits intosony:masterfrom
lo-simon:fix-node-registration

Conversation

@lo-simon
Copy link
Collaborator

@lo-simon lo-simon commented Feb 25, 2026

Change the lambda capture to copy id_type and event_type by value to prevent them from being out of scope in the event task when the condition.wait returns early due to a shutdown.
Thank you to Paul Prayle from Sony for reporting the issue.

…prevent them from being out of scope in the event task when the condition.wait returns early due to a shutdown
@garethsb
Copy link
Contributor

Intrigued by this!

@jonathan-r-thorpe
Copy link
Contributor

@lo-simon was this the only dangling reference found in then code blocks in the whole code base?

@lo-simon
Copy link
Collaborator Author

lo-simon commented Mar 4, 2026

@lo-simon was this the only dangling reference found in then code blocks in the whole code base?

It was fixed before, but somehow it clipped back in after the last merge. And yes, I am not detecting any more possibly dangling references to a temporary warning.

});
request.then([&]
{
condition.notify_all();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What prompted the change from condition.notify_all() to model.notify()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notification code has been moved into the task.next chain. By using model.notify(), the condition.notify_all() wrapper adheres to the same style as in the other behavior threads.

Copy link
Contributor

@jonathan-r-thorpe jonathan-r-thorpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lo-simon lo-simon merged commit 20cb0c5 into sony:master Mar 5, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants